Skip to content

fix(review-suite): grader surface-in-prose defect + v2 s1 re-score comparison - #84

Merged
shaug merged 5 commits into
mainfrom
scott/v2-rescore-s1-correctness-orchestrator
Jul 29, 2026
Merged

fix(review-suite): grader surface-in-prose defect + v2 s1 re-score comparison#84
shaug merged 5 commits into
mainfrom
scott/v2-rescore-s1-correctness-orchestrator

Conversation

@shaug

@shaug shaug commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Summary

Bounded measurement step for #54's evidence gate: re-score
s1-correctness-orchestrator (only; s2/s3 are untouched by #53's
correctness-only change) against the frozen v1 baseline's two confident
misses, dependency-strictness-propagation and stale-claim-release-guard,
after #53's traversal and verification-sufficiency passes landed.

While re-scoring, raw attempts showed 10/10 correctly-reasoned findings on
both cases misclassified as false positives, not because the reviewer was
wrong but because grader.py's surface-hit detection only read structured
location fields and every real attempt named the affected symbol only in
prose. That is a real, pre-existing grader defect (present since the
grader's original commit, not introduced by #53 or by this branch's own
relevance-guard commit) - fixed here, then used to re-grade both the
post-#53 attempts already captured and a fresh pre-#53 (commit 8e4fdbd,
#52's merge) run of the same two cases, for a genuine apples-to-apples
comparison.

Changes

  • review-suite/scripts/evals/grader.py: a referred-path relevance guard
    (referred_relevant_root_cause_ids, combined_recall on grade()) so a
    referred candidate only counts toward the preregistered v2 gate's
    alternative pass path when it concretely names the actual surface, plus a
    real fix (surface_named_in_prose) recognizing a symbol named only in a
    finding's prose - constrained by, and verified against, this repository's
    own probe.partial-claim-wrong-surface calibration boundary. report.py
    surfaces both at the per-case level. GRADER_VERSION 1.0 -> 1.1
    (behavioral, versioned accordingly; every corpus/calibration file's
    declared version bumped to match).
  • review-suite/scripts/evals/regrade.py (new) + just regrade-review-suite: re-grade already-captured raw attempts with the
    current grader, no new executor calls, no new spend.
  • review-suite/evals/v2/ (new files only - nothing under
    baseline/v1/, gate-manifest.json, DECISION-RECORD.md,
    FAILURE-TAXONOMY.md, or audits/ was touched):
    • s1-rescore-frozen-configuration.json - the re-score configuration,
      committed before any scored output was examined.
    • s1-correctness-orchestrator.report.json - post-Add correctness traversal and verification-sufficiency passes #53, 35 attempts,
      re-graded with the fixed grader.
    • pre53-s1-correctness-orchestrator-2case.report.json - a fresh 10
      attempts pre-Add correctness traversal and verification-sufficiency passes #53, graded with the same fixed grader.
    • GRADER-1.1-COMPARABILITY.md - what changed and why a grader-version
      change is a new, non-comparable stratum (extending the existing
      runtime/model-change rule).
    • s1-rescore-pre-post-comparison.md - the four-number comparison,
      non-regression floor check, spend, and a recommendation (not a
      decision) for the repository owner.

Result

Both target cases pass the preregistered v2 scoring gate
(mean_recall >= 0.6 OR guarded mean_combined_recall >= 0.8) in both
eras - mean_combined_recall: 1.0 for all four case x era combinations,
every one of the 20 underlying attempts individually at combined_recall: 1.0. The pre-#53 reviewer already found both defects, every time, before
#53 existed: the frozen v1 baseline's "confident miss" reads as the same
grader artifact this PR fixes, not a demonstrated capability gap. Full
detail, caveats, and the recommendation in
review-suite/evals/v2/s1-rescore-pre-post-comparison.md.

Non-regression floor holds throughout (verdict/finding stability at the v1
floor of 1.0 on both target cases; the three clean controls at zero false
positives; process-isolation-assertion's false-alarm rate improved from
2/5 to 0/5).

Total spend: $4.38 of the $12.00 ceiling (35 post-#53 attempts + 10 fresh
pre-#53 attempts; re-grading both with the fixed grader cost nothing
further).

Review

One review-code-change cycle: solution-simplicity and correctness clean
on the first pass; code-simplicity raised one real finding (a new helper
duplicated an existing one's exact behavior instead of reusing it) -fixed
in the second commit of this PR, re-verified clean.

Test plan

shaug and others added 5 commits July 28, 2026 21:37
## Summary
- `review-suite/scripts/evals/grader.py`: split `match_strength`'s two
  signals into a public `match_signals(expected, finding)` helper returning
  `(surface_hit, signal_hit)`, and record, per referred (partial/ambiguous)
  candidate, whether some referring finding concretely named the candidate's
  actual surface (`candidate_surface_relevant_root_cause_ids`). `grade()` now
  additionally reports `referred_relevant_root_cause_ids` (the subset of
  `referred_root_cause_ids` that passes this guard) and `combined_recall`
  ((matched + relevant-referred) / expected).
- `review-suite/scripts/evals/report.py`: surface the guard at the per-case
  level as `ever_referred_relevant_root_cause_ids` and `mean_combined_recall`,
  reading both defensively via `.get(...)` so a grade dict built before the
  guard existed still reports cleanly.
- Add grader and report unit tests covering a referral that fires on the
  right surface (relevance-guard eligible), a referral that fires on
  formulation prose alone at the wrong location (not eligible), the
  always-relevant ambiguous (two full-match) case, and backward compatibility
  with pre-guard grade dicts.

## Why
- A future re-score of `s1-correctness-orchestrator` needs to grade
  `dependency-strictness-propagation` and `stale-claim-release-guard` against
  a preregistered gate that allows a case to pass on `mean_recall >= 0.6` OR
  a relevance-guarded combined matched+referred rate `>= 0.8`. The existing
  three-way grader already computes `referred_root_cause_ids`, but a referral
  only earned by matching accepted-formulation prose without ever naming the
  right file/function/symbol must not count toward that combined rate - the
  existing tooling had no way to tell the two referral shapes apart before
  scoring, so this was implemented as the smallest additive check on top of
  the grader's existing surface/signal split rather than left to be
  eyeballed during grading.
- `GRADER_VERSION` stays `1.0`: the change is additive only (new fields
  derived from already-computed signals), with no change to any existing
  matched/missed/referred classification or field, so no corpus's declared
  `grader_version` needs to move to keep using it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ctness-orchestrator

## Summary
- Add `review-suite/evals/v2/s1-rescore-frozen-configuration.json`: the
  exact, committed-before-scoring configuration for re-scoring
  `s1-correctness-orchestrator` against the two cases the frozen v1 baseline
  recorded as confident misses (`dependency-strictness-propagation`,
  `stale-claim-release-guard`) after the traversal and
  verification-sufficiency passes landed.
- Pins: origin/main's current head at freeze time
  (85ccf13), the grading-tooling commit
  adding the referred-path relevance guard
  (6195ada), runtime/model
  (claude, CLI 2.1.92, per gate-manifest.json's pin, verified via `claude
  --version`), 5 runs/case, 450s timeout, $12.00 cost ceiling, and the exact
  per-case quality/stability/non-regression gate from #53's issue body's
  "Preregistered v2 scoring gate" section (the repository owner's settled
  refinement of gate-manifest.json's proposal).

## Why
- This is a bounded measurement step for #54: whether #53's two added
  correctness passes actually fixed the misses they were built for.
  Freezing the configuration before examining any scored output - same
  discipline the frozen v1 baseline and gate-manifest.json both already
  apply - is what keeps a later re-score from being tuned to a result seen
  in advance.
- Only s1-correctness-orchestrator is in scope: s2/s3 are solution- and
  code-simplicity strata untouched by #53's correctness-only change, so
  re-scoring them would spend real money against this measurement's own
  ceiling to re-confirm a null result already expected by
  gate-manifest.json's non-regression floor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e hit

## Summary
- `review-suite/scripts/evals/grader.py`: add `surface_named_in_prose(expected,
  finding)`, which checks whether the expectation's whole `surface` field,
  normalized to a phrase, appears as a contiguous substring of the finding's
  prose (`finding_text`: `rule`/`concern`/`impact`/`proposed_change`/
  `expected_effect`, `evidence[].detail`). `match_signals`'s `surface_hit` is
  now `finding_surfaces` (structured `location` fields, unchanged) OR this new
  phrase check - purely additive, no existing hit is removed.
- Bump `GRADER_VERSION` to `1.1` and every corpus's declared `grader_version`
  to match (`review-suite/evals/corpus/corpus.json`,
  `review-suite/evals/calibration/rollback-guidance-render.json`, and every
  `review-suite/evals/strata/*/corpus.json`) - a behavioral grading change,
  unlike the purely-additive-fields commit that preceded this one on this
  branch.
- Add `review-suite/scripts/evals/regrade.py` (+
  `review-suite/scripts/tests/test_eval_regrade.py`): re-grade already-
  captured raw attempts with the current grader, from a prior run's
  `--attempts-out` and `--artifact-dir`, without launching any executor
  process or spending anything new. Exposed as `just regrade-review-suite`.
- Add grader tests for both the fix and its own precision boundary: a symbol
  named only in prose is now a surface hit
  (`test_a_symbol_named_only_in_prose_is_a_surface_hit`), but a lone common
  word incidentally shared with a multi-word surface is not
  (`test_a_lone_common_word_shared_with_the_surface_is_not_a_surface_hit`).

## Why
- Reading raw real-runtime `s1-correctness-orchestrator` attempts for
  `dependency-strictness-propagation` and `stale-claim-release-guard` found
  10/10 correctly-reasoned findings misclassified `unexpected` (false
  positive) instead of even `referred`. Every attempt names the exact
  expected symbol (`dependency_finalized`, `_release`) only in prose, while
  every structured `location`/`evidence[].location` field carries a bare file
  path with no symbol in it - `dependencies.py` (plural, from the path) never
  token-matches `dependency` (singular, from the surface field) under the
  prior location-only, exact-token surface check, so `surface_hit` was false
  even though a human reading the same prose immediately recognizes the
  correct root cause.
- A first attempt at this fix (token-set union of all prose into
  `finding_surfaces`) over-corrected: it silently turned this repository's own
  `probe.partial-claim-wrong-surface` calibration boundary (a deliberately
  wrong-surface, partially-correct claim that must stay referred, not
  matched) into a full match, because its prose incidentally contains the
  single common word "guidance" - one token of the multi-word surface
  `render_rollback_guidance` - without ever naming the surface as a whole
  phrase. Requiring the *whole* normalized surface phrase to appear (mirroring
  how `_signal_match` already does phrase-substring matching for
  formulations) fixes the real case while leaving that calibration boundary
  intact; `just test` catching this immediately is exactly what the
  calibration suite exists for.
- This is a real behavioral change to grading semantics, not merely new
  derived fields, so it is versioned as such: any score computed under
  `GRADER_VERSION` `1.0` (including the entire frozen v1 baseline) is not
  directly comparable to a score computed under `1.1`. Documented in a new,
  separate file under `review-suite/evals/v2/` in a following commit - this
  commit does not touch `gate-manifest.json`, `DECISION-RECORD.md`,
  `FAILURE-TAXONOMY.md`, or any audit file, all still frozen and not owned by
  this change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…or s1

## Summary
- `review-suite/evals/v2/s1-correctness-orchestrator.report.json`: the
  post-#53 (commit `2c671fd`) 35-attempt run, re-graded with `GRADER_VERSION`
  `1.1` via `regrade.py` (no new executor calls).
- `review-suite/evals/v2/pre53-s1-correctness-orchestrator-2case.report.json`:
  a fresh 10-attempt run (5 runs each) of `dependency-strictness-propagation`
  and `stale-claim-release-guard` against the pre-#53 correctness lens
  (commit `8e4fdbd`, #52's merge), graded with the same fixed grader, from a
  temporary 2-case subset corpus built from the two cases' unmodified
  packet/expectation/provenance files.
- `review-suite/evals/v2/GRADER-1.1-COMPARABILITY.md`: what the grader fix
  changed, why the first attempted fix was too loose, and the comparability
  rule it creates (a `GRADER_VERSION` change is a new, non-comparable
  stratum, same as a runtime/model change).
- `review-suite/evals/v2/s1-rescore-pre-post-comparison.md`: the plain
  comparison - four numbers (pre-#53/post-#53 x the two target cases),
  non-regression floor check, spend, and a recommendation (not a decision)
  for the repository owner.

## Result
Both target cases pass the preregistered v2 scoring gate
(`mean_recall >= 0.6` OR guarded `mean_combined_recall >= 0.8`) in both
eras: `mean_combined_recall: 1.0` for all four (case x era) combinations,
every one of the 20 underlying attempts individually scoring
`combined_recall: 1.0`. The non-regression floor holds throughout. Total
spend across the whole measurement (35 post-#53 + 10 pre-#53 attempts):
$4.38 of the $12.00 ceiling.

This shows the pre-#53 reviewer already found both defects, every time,
before #53 existed - the frozen v1 baseline's "confident miss" on these two
cases reads as a grader artifact (the same location-only surface-matching
blind spot `GRADER-1.1-COMPARABILITY.md` documents), not a demonstrated
reviewer capability gap. Neither `baseline/v1/` nor `gate-manifest.json`,
`DECISION-RECORD.md`, `FAILURE-TAXONOMY.md`, or any audit file is modified;
this adds new files alongside them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Summary
- `surface_named_in_prose` now delegates to the existing `_signal_match`
  (called with a single-item `[expected.get("surface", "")]` formulations
  list) instead of reimplementing the same normalize-then-substring check a
  second time.

## Why
- Found in review-code-change's code-simplicity pass on this branch:
  `surface_named_in_prose`'s body was behaviorally identical to
  `_signal_match([surface], text)` in every case checked (the real
  dependency_finalized/_release matches, the render_rollback_guidance
  wrong-surface calibration case, and both empty/missing surface fields),
  including the same falsy-formulation guard that makes an empty `surface`
  never match. Two separately-named implementations of the same rule is one
  more concept than a maintainer needs to hold, and a future change to the
  matching rule would otherwise have to be made in both places.
- Purely internal: no observable behavior changes. `just test` (252 tests)
  passes unchanged before and after.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shaug
shaug merged commit a247d47 into main Jul 29, 2026
1 check passed
@shaug
shaug deleted the scott/v2-rescore-s1-correctness-orchestrator branch July 29, 2026 11:59
shaug added a commit that referenced this pull request Jul 29, 2026
…e surface-in-prose defect (#85)

## Summary
- Confirm no v1 raw attempt data survives for s2-solution-simplicity-lens or
  s3-code-simplicity-lens (searched machine-wide; only unrelated pilot-stratum
  artifacts remain), matching s1's documented pattern.
- Run both strata fresh (40 attempts, 5 runs/case) under the runtime/model
  pin unchanged (claude 2.1.92, claude-opus-4-6[1m]) and the fixed grader
  (1.1, commit a247d47).
- Re-grade the same fresh raw attempts with the pre-fix grader (85ccf13) as a
  direct counterfactual: three of the four material cases were already
  correctly routed to `referred` under the old grader and show no material
  difference; `setup-service-path-gateway` is materially affected by the same
  surface-named-only-in-prose defect s1's two cases had, at the
  referred-vs-false-positive boundary rather than a full blind miss.
- Confirm the four clean/negative controls are unchanged (zero false
  positives, perfect stability, both eras).

## Why
- The grader defect fixed in #84 was known to reach s1's two target cases;
  whether it also reached s2/s3 (whose material cases already showed
  non-empty referred lists in v1, unlike s1's fully-blind cases) had never
  been checked. This measurement checks it directly instead of assuming the
  answer, and finds a real but smaller-scope effect on one case
  (`setup-service-path-gateway`'s false-positive rate) rather than a clean
  "unaffected" result.

Spend: $2.15 of 40 attempts, against this task's $5.00 ceiling.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant